Skip to content

GH-35480: [MATLAB] Add abstract MATLAB base class called arrow.array.Array#35491

Merged
kou merged 4 commits into
apache:mainfrom
mathworks:GH-35480
May 11, 2023
Merged

GH-35480: [MATLAB] Add abstract MATLAB base class called arrow.array.Array#35491
kou merged 4 commits into
apache:mainfrom
mathworks:GH-35480

Conversation

@sgilmore10

@sgilmore10 sgilmore10 commented May 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Rationale for this change

It will be helpful for sharing the implementation of common functionality if there is an abstract base class from which the MATLAB arrow.array.[Type]Array classes can inherit.

What changes are included in this PR?

  1. Added abstract base class arrow.array.Array
  2. Changed arrow.array.Float64Array to inherit from arrow.array.Array
  3. Added Length property on arrow.array.Array

Are these changes tested?

  1. Added a test point for the Length property in tFloat64Array.m.
  2. Qualified locally on macOS and linux.

Are there any user-facing changes?

Yes, there is now a Length property on arrow.array.Float64Array

Future Directions

We will continue to build out the concrete Array subclasses.

@sgilmore10 sgilmore10 changed the title GH-35480: [MATLAB] [3:05 PM] Add abstract MATLAB base class called arrow.array.Array GH-35480: [MATLAB] Add abstract MATLAB base class called arrow.array.Array May 8, 2023
@github-actions

github-actions Bot commented May 8, 2023

Copy link
Copy Markdown

@github-actions github-actions Bot added the awaiting review Awaiting review label May 8, 2023
@sgilmore10 sgilmore10 mentioned this pull request May 8, 2023
Merged
sgilmore10 and others added 3 commits May 10, 2023 09:23
@sgilmore10 @kevingurney
Add abstract base class arrow.array.Array
b3d1ea6 
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
@sgilmore10 @kevingurney
1. Add abstract MATLAB base class arrow.array.Array
c32d901 
2. Implement Length()
3. Make arrow.array.Float64Array inherit from arrow.array.Arrat

Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
@kevingurney @sgilmore10
Add test point for arrow.array.Array.Length property.
8645e1b 
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
@sgilmore10 sgilmore10 marked this pull request as ready for review May 10, 2023 14:02
@sgilmore10 sgilmore10 requested a review from assignUser as a code owner May 10, 2023 14:02
@sgilmore10

sgilmore10 commented May 10, 2023

Copy link
Copy Markdown
Member Author

Marking PR as read for review.

@kevingurney kevingurney mentioned this pull request May 11, 2023
Merged
kou
kou approved these changes May 11, 2023
View reviewed changes

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels May 11, 2023
@kou kou merged commit cdefbb8 into apache:main May 11, 2023
@ursabot

ursabot commented May 13, 2023

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 14f9bf9 and contender = cdefbb8. cdefbb8 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️2.27% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.25% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.42% ⬆️0.03%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] cdefbb8f ec2-t3-xlarge-us-east-2
[Finished] cdefbb8f test-mac-arm
[Finished] cdefbb8f ursa-i9-9960x
[Finished] cdefbb8f ursa-thinkcentre-m75q
[Finished] 14f9bf92 ec2-t3-xlarge-us-east-2
[Finished] 14f9bf92 test-mac-arm
[Finished] 14f9bf92 ursa-i9-9960x
[Finished] 14f9bf92 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

ArgusLi pushed a commit to Bit-Quill/arrow that referenced this pull request May 15, 2023
@sgilmore10 @kevingurney @ArgusLi
apacheGH-35480: [MATLAB] Add abstract MATLAB base class called `arrow…
36398da 
….array.Array` (apache#35491)

### Rationale for this change

It will be helpful for sharing the implementation of common functionality if there is an abstract base class from which the MATLAB `arrow.array.[Type]Array` classes can inherit. 

### What changes are included in this PR?

1. Added abstract base class `arrow.array.Array`
2. Changed `arrow.array.Float64Array` to inherit from `arrow.array.Array`
3. Added `Length` property on `arrow.array.Array`

### Are these changes tested?

1. Added a test point for the `Length` property in `tFloat64Array.m`.
2. Qualified locally on macOS and linux.

### Are there any user-facing changes?

Yes, there is now a `Length` property on `arrow.array.Float64Array`

### Future Directions

We will continue to build out the concrete Array subclasses. 
* Closes: apache#35480

Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kevingurney kevingurney mentioned this pull request Aug 16, 2023
Closed
4 tasks
@kevingurney kevingurney deleted the GH-35480 branch August 21, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kou kou kou approved these changes

@assignUser assignUser Awaiting requested review from assignUser

Assignees

No one assigned

Labels

awaiting merge Awaiting merge Component: MATLAB

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[MATLAB] Add abstract MATLAB base class called arrow.array.Array

4 participants

@sgilmore10 @ursabot @kou @kevingurney